packages/web/src/features/fileTree/components/pureFileTreePanel.tsx (1)

54-60: Avoid mutating React state inside transformTree

currentNode.isCollapsed = isCollapsed directly mutates a node that still belongs to the previous state tree.
Although you later spread-copy the node, the mutation happens first, so the original tree is mutated before React receives the new tree, violating React’s immutability contract and risking subtle bugs (e.g. missed re-renders in memoised children).

-            if (currentNode.path === path) {
-                currentNode.isCollapsed = isCollapsed;
-            }
-            return currentNode;
+            if (currentNode.path === path) {
+                /* return a new object – never mutate the existing one */
+                return { ...currentNode, isCollapsed };
+            }
+            return currentNode;

packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx (1)

2-6: Remove unused imports from client-side version.

These imports are no longer needed after the conversion to a server component:

• useBrowseParams (now receives props)
• useQuery (now uses direct await)
• useDomain (now receives domain as prop)
• Loader2 (loading handled by Suspense in parent)

-import { useBrowseParams } from "@/app/[domain]/browse/hooks/useBrowseParams";
-import { useQuery } from "@tanstack/react-query";
-import { getFileSource } from "@/features/search/fileSourceApi";
-import { useDomain } from "@/hooks/useDomain";
-import { Loader2 } from "lucide-react";
+import { getFileSource } from "@/features/search/fileSourceApi";

packages/web/src/features/fileTree/components/pureFileTreePanel.tsx (2)

95-98: Redundant key prop duplication

Both the React.Fragment (line 95) and the nested FileTreeItemComponent (line 97) receive the same key={node.path}.
The extra key on the child component is superfluous and can be safely removed to keep the markup tidy.

-<React.Fragment key={node.path}>
-    <FileTreeItemComponent
-        key={node.path}
+<React.Fragment key={node.path}>
+    <FileTreeItemComponent

---

112-113: renderTree dependency list may cause unnecessary re-creations

renderTree depends on scrollAreaRef (passed to children) but the ref is omitted from the dependency array.
While useRef objects are stable, future changes (e.g. swapping to useState or replacing the ref) would silently break memoisation.
For completeness and future-proofing, include it:

-    }, [path, onNodeClicked]);
+    }, [path, onNodeClicked, scrollAreaRef]);

packages/web/src/app/[domain]/browse/hooks/utils.ts (2)

3-3: Fix typo in variable name.

The variable name sentinalIndex should be sentinelIndex (missing 'e').

-    const sentinalIndex = pathParam.search(/\/-\/(tree|blob)\//);
+    const sentinelIndex = pathParam.search(/\/-\/(tree|blob)\//);

You'll also need to update the usage on lines 4, 8, and 13.

---

16-16: Fix typo in comment.

The comment mentions decodedURIComponent but should be decodeURIComponent.

-        // @note: decodedURIComponent is needed here incase the path contains a space.
+        // @note: decodeURIComponent is needed here in case the path contains a space.

packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx (1)

25-27: Enhance error handling with more descriptive messages

The current error handling doesn't provide enough context about which operation failed or why.

     if (isServiceError(folderContentsResponse) || isServiceError(repoInfoResponse)) {
-        return <div>Error loading tree preview</div>
+        const errorMessage = isServiceError(repoInfoResponse) 
+            ? `Failed to load repository information for ${repoName}`
+            : `Failed to load folder contents at ${path}`;
+        return <div className="p-4 text-red-500">{errorMessage}</div>
     }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx (2)

26-28: Enhance error handling with more specific error information

The current error handling is quite generic. Consider providing more specific error messages or logging for debugging purposes.

-    if (isServiceError(folderContentsResponse) || isServiceError(repoInfoResponse)) {
-        return <div>Error loading tree preview</div>
+    if (isServiceError(folderContentsResponse) || isServiceError(repoInfoResponse)) {
+        const folderError = isServiceError(folderContentsResponse) ? folderContentsResponse.error : null;
+        const repoError = isServiceError(repoInfoResponse) ? repoInfoResponse.error : null;
+        console.error('TreePreviewPanel error:', { folderError, repoError });
+        return <div>Error loading tree preview. Please try again.</div>

---

16-47: Consider user experience implications of server-side rendering

While the server component refactor fixes the infinite loading issue, users will now see a blank state until the entire component loads. This trade-off is acceptable given the fix, but consider implementing loading UI at a higher level in the component hierarchy.

For future improvements, you might want to:

1. Add loading indicators at the page level
2. Consider implementing streaming or progressive loading for large folder contents
3. Explore Next.js's built-in prefetching mechanisms as mentioned in the PR description

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

packages/web/src/app/[domain]/browse/hooks/utils.test.ts (3)

48-49: Remove unnecessary empty lines.

There are extra empty lines that should be cleaned up for better code consistency.

 describe('blob paths', () => {
-
-
     it('should parse blob path with file', () => {

---

158-168: Make error assertions more specific.

The error tests use generic .toThrow() without verifying the specific error messages. Consider testing the exact error messages to ensure the function provides meaningful feedback.

 it('should throw error for blob path with trailing slash and no path', () => {
     expect(() => {
         getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt@HEAD/-/blob/');
-    }).toThrow();
+    }).toThrow('Invalid browse pathname: "github.com/sourcebot-dev/zoekt@HEAD/-/blob/" - expected to contain a path for blob type');
 });

 it('should throw error for blob path without trailing slash and no path', () => {
     expect(() => {
         getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt@HEAD/-/blob');
-    }).toThrow();
+    }).toThrow('Invalid browse pathname: "github.com/sourcebot-dev/zoekt@HEAD/-/blob" - expected to contain a path for blob type');
 });

---

170-186: Make remaining error assertions more specific.

For consistency and better test reliability, these error tests should also verify specific error messages.

 it('should throw error for invalid pattern - missing /-/', () => {
     expect(() => {
         getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt@HEAD/tree/');
-    }).toThrow();
+    }).toThrow('Invalid browse pathname: "github.com/sourcebot-dev/zoekt@HEAD/tree/" - expected to contain "/-/(tree|blob)/" pattern');
 });

 it('should throw error for invalid pattern - missing tree/blob', () => {
     expect(() => {
         getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt@HEAD/-/invalid/');
-    }).toThrow();
+    }).toThrow('Invalid browse pathname: "github.com/sourcebot-dev/zoekt@HEAD/-/invalid/" - expected to contain "/-/(tree|blob)/" pattern');
 });

 it('should throw error for completely invalid format', () => {
     expect(() => {
         getBrowseParamsFromPathParam('invalid-path');
-    }).toThrow();
+    }).toThrow('Invalid browse pathname: "invalid-path" - expected to contain "/-/(tree|blob)/" pattern');
 });

 it('should throw error for empty string', () => {
     expect(() => {
         getBrowseParamsFromPathParam('');
-    }).toThrow();
+    }).toThrow('Invalid browse pathname: "" - expected to contain "/-/(tree|blob)/" pattern');
 });

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro